Fix deps with `cargo test --all` and doctests

This commit fixes `cargo test --all` with the way we ship libraries to `rustdoc
--test`. I'm... not entirely sure what the previous incarnation was doing but
the current organization is to interpret `compilation.libraries` as a mapping
from a package to the list of crates it needs to link to test.

This updates the support in `cargo_rustc/mod.rs` to create the map appropriately
and tweaks the loop in `cargo_test.rs` as well.

Closes rust-lang/rust#39879

diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs
index 510b142db8f516483fc09c83d414027c0cc11add..60f62eada544ef8449c21e1b637a202d8cddf21c 100644 (file)
--- a/src/cargo/ops/cargo_rustc/compilation.rs
+++ b/src/cargo/ops/cargo_rustc/compilation.rs
@@ -8,11 +8,9 @@ use util::{self, CargoResult, Config, ProcessBuilder, process, join_paths};
 
 /// A structure returning the result of a compilation.
 pub struct Compilation<'cfg> {
-    /// All libraries which were built for a package.
-    ///
-    /// This is currently used for passing --extern flags to rustdoc tests later
-    /// on.
-    pub libraries: HashMap<PackageId, Vec<(Target, PathBuf)>>,
+    /// A mapping from a package to the list of libraries that need to be
+    /// linked when working with that package.
+    pub libraries: HashMap<PackageId, HashSet<(Target, PathBuf)>>,
 
     /// An array of all tests created during this compilation.
     pub tests: Vec<(Package, String, PathBuf)>,

diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
index 620ff267f2e9d6eef3884bb784655c14c53c9ad2..14ebcc28c1f44bf930d4cbb9890801094f99d9b4 100644 (file)
--- a/src/cargo/ops/cargo_rustc/mod.rs
+++ b/src/cargo/ops/cargo_rustc/mod.rs
@@ -153,33 +153,31 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
                 cx.compilation.binaries.push(bindst);
             } else if unit.target.is_lib() {
                 let pkgid = unit.pkg.package_id().clone();
-                cx.compilation.libraries.entry(pkgid).or_insert(Vec::new())
-                  .push((unit.target.clone(), dst));
+                cx.compilation.libraries.entry(pkgid).or_insert(HashSet::new())
+                  .insert((unit.target.clone(), dst));
             }
+        }
+
+        for dep in cx.dep_targets(unit)?.iter() {
             if !unit.target.is_lib() { continue }
 
-            // Include immediate lib deps as well
-            for unit in cx.dep_targets(unit)?.iter() {
-                if unit.profile.run_custom_build {
-                    let out_dir = cx.build_script_out_dir(unit).display().to_string();
-                    cx.compilation.extra_env.entry(unit.pkg.package_id().clone())
-                      .or_insert(Vec::new())
-                      .push(("OUT_DIR".to_string(), out_dir));
-                }
+            if dep.profile.run_custom_build {
+                let out_dir = cx.build_script_out_dir(dep).display().to_string();
+                cx.compilation.extra_env.entry(dep.pkg.package_id().clone())
+                  .or_insert(Vec::new())
+                  .push(("OUT_DIR".to_string(), out_dir));
+            }
 
-                let pkgid = unit.pkg.package_id();
-                if !unit.target.is_lib() { continue }
-                if unit.profile.doc { continue }
-                if cx.compilation.libraries.contains_key(pkgid) {
-                    continue
-                }
+            if !dep.target.is_lib() { continue }
+            if dep.profile.doc { continue }
 
-                let v = cx.target_filenames(unit)?;
-                let v = v.into_iter().map(|(f, _, _)| {
-                    (unit.target.clone(), f)
-                }).collect::<Vec<_>>();
-                cx.compilation.libraries.insert(pkgid.clone(), v);
-            }
+            let v = cx.target_filenames(dep)?;
+            cx.compilation.libraries
+                .entry(unit.pkg.package_id().clone())
+                .or_insert(HashSet::new())
+                .extend(v.into_iter().map(|(f, _, _)| {
+                    (dep.target.clone(), f)
+                }));
         }
 
         let feats = cx.resolve.features(&unit.pkg.package_id());

diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs
index 145559248883105937dd1b0cc1caf5d29241f136..c8abc0ed978d914526ffc0782b3762c9567298b1 100644 (file)
--- a/src/cargo/ops/cargo_test.rs
+++ b/src/cargo/ops/cargo_test.rs
@@ -151,27 +151,26 @@ fn run_doc_tests(options: &TestOptions,
                 }
             }
 
-            for (_, libs) in compilation.libraries.iter() {
-                for &(ref target, ref lib) in libs.iter() {
-                    // Note that we can *only* doctest rlib outputs here.  A
-                    // staticlib output cannot be linked by the compiler (it just
-                    // doesn't do that). A dylib output, however, can be linked by
-                    // the compiler, but will always fail. Currently all dylibs are
-                    // built as "static dylibs" where the standard library is
-                    // statically linked into the dylib. The doc tests fail,
-                    // however, for now as they try to link the standard library
-                    // dynamically as well, causing problems. As a result we only
-                    // pass `--extern` for rlib deps and skip out on all other
-                    // artifacts.
-                    if lib.extension() != Some(OsStr::new("rlib")) &&
-                       !target.for_host() {
-                        continue
-                    }
-                    let mut arg = OsString::from(target.crate_name());
-                    arg.push("=");
-                    arg.push(lib);
-                    p.arg("--extern").arg(&arg);
+            let libs = &compilation.libraries[package.package_id()];
+            for &(ref target, ref lib) in libs.iter() {
+                // Note that we can *only* doctest rlib outputs here.  A
+                // staticlib output cannot be linked by the compiler (it just
+                // doesn't do that). A dylib output, however, can be linked by
+                // the compiler, but will always fail. Currently all dylibs are
+                // built as "static dylibs" where the standard library is
+                // statically linked into the dylib. The doc tests fail,
+                // however, for now as they try to link the standard library
+                // dynamically as well, causing problems. As a result we only
+                // pass `--extern` for rlib deps and skip out on all other
+                // artifacts.
+                if lib.extension() != Some(OsStr::new("rlib")) &&
+                   !target.for_host() {
+                    continue
                 }
+                let mut arg = OsString::from(target.crate_name());
+                arg.push("=");
+                arg.push(lib);
+                p.arg("--extern").arg(&arg);
             }
 
             config.shell().verbose(|shell| {

diff --git a/tests/test.rs b/tests/test.rs
index dbeab28c741fd4a636636171f8b064b245ad6a3e..6e3e5c4646cb5882c6d74dcc0390d756a8a87d81 100644 (file)
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -2624,3 +2624,45 @@ fn test_many_targets() {
                     .with_stderr_contains("[RUNNING] `rustc --crate-name a examples[/]a.rs [..]`")
                     .with_stderr_contains("[RUNNING] `rustc --crate-name b examples[/]b.rs [..]`"))
 }
+
+#[test]
+fn doctest_and_registry() {
+    let p = project("workspace")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "a"
+            version = "0.1.0"
+
+            [dependencies]
+            b = { path = "b" }
+            c = { path = "c" }
+
+            [workspace]
+        "#)
+        .file("src/lib.rs", "")
+        .file("b/Cargo.toml", r#"
+            [project]
+            name = "b"
+            version = "0.1.0"
+        "#)
+        .file("b/src/lib.rs", "
+            /// ```
+            /// b::foo();
+            /// ```
+            pub fn foo() {}
+        ")
+        .file("c/Cargo.toml", r#"
+            [project]
+            name = "c"
+            version = "0.1.0"
+
+            [dependencies]
+            b = "0.1"
+        "#)
+        .file("c/src/lib.rs", "");
+
+    Package::new("b", "0.1.0").publish();
+
+    assert_that(p.cargo_process("test").arg("--all").arg("-v"),
+                execs().with_status(0));
+}